-
-
Notifications
You must be signed in to change notification settings - Fork 475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(graphql_parser): parse enum extension #3044
feat(graphql_parser): parse enum extension #3044
Conversation
CodSpeed Performance ReportMerging #3044 will not alter performanceComparing Summary
|
2b6e286
to
d1de19c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
let m = p.start(); | ||
|
||
p.bump(T![extend]); | ||
p.bump(T![enum]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p.bump(T![enum]); | |
p.expect(T![enum]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is truly necessary, as this parsing rule should only be called in one place (here), and we must have already checked for the presence of both tokens before advancing. Correct me if you think otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it’s not a blocker, but it’s good practice for the parser functions to validate their preconditions instead of assuming to be called correctly. It wouldn’t change anything functionally now, but it makes the contract more explicit and brings some extra robustness in the face of future changes to the parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to merge once we address Arend suggestion (even in another PR if that's okay)
Summary
Parse GraphQL enum extensions (link). Once this is completed, we will only have input object extensions (link) remaining before the GraphQL parser is ready for benchmarking.
Test Plan
All tests should pass